-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
set_refund_for_current_tx #2361
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Locally, the changes seem to allow the load test set up to complete, and the load test itself now runs fine 🎉 There are still issues with batch executor unit tests though, like extra storage logs and L2-to-L1 logs being produced by the new VM.
@@ -145,14 +146,15 @@ impl<S: ReadStorage> Vm<S> { | |||
.as_u64(); | |||
|
|||
let pubdata_published = self.inner.world_diff.pubdata.0 as u32; | |||
//pubdata_published -= pubdata_before; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove if this code isn't needed. BTW, how would pubdata computations work if the entire batch is executed (i.e., execution_mode == VmExecutionMode::Batch
)? I'd think that pubdata_before
needs to be updated somewhere in run()
(in TxHasEnded
hook handler?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed it to be updated here on the spot, as for now i do not have a better theory about where it should be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's correct (it would probably lead to the previous issue of pubdata_published
for a transaction including pubdata produced by the bootloader / system contracts after executing the previous transaction), but let's fix this sequentially.
What ❔
Fixes refund-related bugs in the glue code for the new VM:
pubdata_published
as a difference with the previous transaction